Skip to content

Fix setFullScreenMode on talk viewer opening #1331

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
skjnldsv merged 1 commit into from
Aug 24, 2022
Merged

Fix setFullScreenMode on talk viewer opening #1331

skjnldsv merged 1 commit into from
Aug 24, 2022

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Aug 17, 2022 •
edited
Loading

Because Talk is doing some shenanigans to fake the Files Sidebar

Uncaught (in promise) TypeError: OCA.Files.Sidebar.setFullScreenMode is not a function
    at viewer-main.js:2
    at u (talk-main.js:1)
    at Generator._invoke (talk-main.js:1)
    at Generator.next (talk-main.js:1)
    at Rw (viewer-main.js:2)
    at a (viewer-main.js:2)
    at viewer-main.js:2
    at new Promise (<anonymous>)
    at viewer-main.js:2
    at a.openFile (viewer-main.js:2)

@skjnldsv skjnldsv added bug Something isn't working 3. to review Waiting for reviews labels Aug 17, 2022
@skjnldsv skjnldsv self-assigned this Aug 17, 2022
@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 17, 2022

/backport to stable24

@backportbot-nextcloud backportbot-nextcloud bot added the backport-request Pending backport by the backport-bot label Aug 17, 2022
@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 17, 2022

/rebase

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 17, 2022

/compile amend /

@skjnldsv skjnldsv requested a review from szaimen August 17, 2022 13:11
@nickvergessen nickvergessen requested review from marcoambrosini and removed request for nickvergessen August 17, 2022 13:51
danxuliu
danxuliu reviewed Aug 19, 2022
View reviewed changes
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have reproduction steps? I have never seen that error (and I also tried to reproduce it without success), and it should not happen, as the fake sidebar has a setFullScreenMode function.

If the issue is caused by Talk I think it would be better to fix it in Talk instead (if it is possible, which I do not know :-) ).

@skjnldsv
Copy link
Member Author

skjnldsv commented Aug 19, 2022

Maybe I was too much behind lol
In any case, this is good sanity check ;)

@skjnldsv skjnldsv requested a review from danxuliu August 19, 2022 07:57
danxuliu
danxuliu approved these changes Aug 19, 2022
View reviewed changes
Copy link
Member

@danxuliu danxuliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I was too much behind lol

More than a year, it is surprising that Talk even loaded :-O

In any case, this is good sanity check ;)

Then typeof OCA?.Files?.Sidebar?.setFullScreenMode === 'function'? Or that is too much? :-P

Anyway, fine by me to keep the old code, to check if setFullScreenMode exists or to check if setFullScreenMode is a function 👍

@skjnldsv skjnldsv closed this Aug 19, 2022
@skjnldsv skjnldsv deleted the fix/sidebar-talk-open branch August 19, 2022 10:47
@skjnldsv skjnldsv restored the fix/sidebar-talk-open branch August 19, 2022 10:47
@skjnldsv skjnldsv reopened this Aug 19, 2022
@skjnldsv skjnldsv force-pushed the fix/sidebar-talk-open branch from 37baf15 to 60c475e Compare August 24, 2022 14:09
@skjnldsv skjnldsv force-pushed the fix/sidebar-talk-open branch from 60c475e to 2732f1b Compare August 24, 2022 14:11
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 24, 2022
@skjnldsv skjnldsv merged commit fcd357d into master Aug 24, 2022
@skjnldsv skjnldsv deleted the fix/sidebar-talk-open branch August 24, 2022 14:29
@backportbot-nextcloud
Copy link

backportbot-nextcloud bot commented Aug 24, 2022

The backport to stable24 failed. Please do this backport manually.

@blizzz blizzz mentioned this pull request Aug 24, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@danxuliu danxuliu danxuliu approved these changes

@szaimen szaimen Awaiting requested review from szaimen

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees

@skjnldsv skjnldsv

Labels

4. to release Ready to be released and/or waiting for tests to finish backport-request Pending backport by the backport-bot bug Something isn't working

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@skjnldsv @danxuliu